[Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

[Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

[Stefan Hajnoczi]

There are several places where QEMU accidentally relies on the O_NONBLOCK state
of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the QMP
API whenever getfd or fdset_add_fd are used!

Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
be hidden from QMP clients.

This patch series addresses this in 3 steps:

1. Fix callers of monitor_handle_fd_param(), monitor_fdset_get_fd(), and
   monitor_get_fd() that depend on O_NONBLOCK being set.  Luckily there are
   only two instances and they are fixed in Patches 1 & 2.

2. Rename socket_set_nonblock() to qemu_set_nonblock() just like
   qemu_set_cloexec().  This makes code cleaner when working with arbitrary
   file descriptors that may not be sockets.  See Patch 3.

3. Clear O_NONBLOCK when a chardev receives file descriptors.  From now on QEMU
   can assume that passed file descriptors are in blocking mode.  Simply use
   qemu_set_nonblock(fd) if you want to enable O_NONBLOCK.  See Patch 4.

This fixes live migration with recent libvirt.  Libvirt checks if QEMU supports
file descriptor passing and, if yes, hands QEMU a socket with O_NONBLOCK set.
The migrate fd:<foo> code assumes the socket is in blocking mode.  The result
is a corrupted migration stream.  For more info on this bug, see:

https://bugzilla.redhat.com/show_bug.cgi?id=923124

Note that Michal Privoznik <mprivozn@redhat.com> also sent a libvirt patch so
that old QEMUs work with new libvirts:

https://www.redhat.com/archives/libvir-list/2013-March/msg01486.html

My patch series fixes the QMP API and allows old libvirts to work again with
new QEMUs.

v2:
 * Rename socket_set_nonblock() in Patch 1 to avoid code churn [eblake]
 * Avoid qemu_set_block(-1) calls that clobber errno [quintela]

Stefan Hajnoczi (4):
  oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()
  net: ensure "socket" backend uses non-blocking fds
  qemu-socket: set passed fd non-blocking in socket_connect()
  chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors

 block/nbd.c            |  2 +-
 block/sheepdog.c       |  2 +-
 include/qemu/sockets.h |  4 ++--
 migration.c            |  2 +-
 nbd.c                  |  8 ++++----
 net/socket.c           | 13 +++++++++----
 qemu-char.c            | 11 +++++++----
 savevm.c               |  2 +-
 slirp/misc.c           |  2 +-
 slirp/tcp_subr.c       |  4 ++--
 ui/vnc.c               |  2 +-
 util/oslib-posix.c     |  4 ++--
 util/oslib-win32.c     |  4 ++--
 util/qemu-sockets.c    |  5 +++--
 14 files changed, 37 insertions(+), 28 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH v2 1/4] oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()

[Stefan Hajnoczi]

The fcntl(fd, F_SETFL, O_NONBLOCK) flag is not specific to sockets.
Rename to qemu_set_nonblock() just like qemu_set_cloexec().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nbd.c            | 2 +-
 block/sheepdog.c       | 2 +-
 include/qemu/sockets.h | 4 ++--
 migration.c            | 2 +-
 nbd.c                  | 8 ++++----
 net/socket.c           | 6 +++---
 qemu-char.c            | 8 ++++----
 savevm.c               | 2 +-
 slirp/misc.c           | 2 +-
 slirp/tcp_subr.c       | 4 ++--
 ui/vnc.c               | 2 +-
 util/oslib-posix.c     | 4 ++--
 util/oslib-win32.c     | 4 ++--
 util/qemu-sockets.c    | 4 ++--
 14 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 3d711b2..eff683c 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -415,7 +415,7 @@ static int nbd_establish_connection(BlockDriverState *bs)
 
     /* Now that we're connected, set the socket to be non-blocking and
      * kick the reply mechanism.  */
-    socket_set_nonblock(sock);
+    qemu_set_nonblock(sock);
     qemu_aio_set_fd_handler(sock, nbd_reply_ready, NULL,
                             nbd_have_request, s);
 
diff --git a/block/sheepdog.c b/block/sheepdog.c
index bb67c4c..987018e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -471,7 +471,7 @@ static int connect_to_sdog(BDRVSheepdogState *s)
         qerror_report_err(err);
         error_free(err);
     } else {
-        socket_set_nonblock(fd);
+        qemu_set_nonblock(fd);
     }
 
     return fd;
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index d225f6d..c5174d7 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -37,8 +37,8 @@ int qemu_socket(int domain, int type, int protocol);
 int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
 int socket_set_cork(int fd, int v);
 int socket_set_nodelay(int fd);
-void socket_set_block(int fd);
-void socket_set_nonblock(int fd);
+void qemu_set_block(int fd);
+void qemu_set_nonblock(int fd);
 int send_all(int fd, const void *buf, int len1);
 int recv_all(int fd, void *buf, int len1, bool single_read);
 
diff --git a/migration.c b/migration.c
index 7fb2147..3b4b467 100644
--- a/migration.c
+++ b/migration.c
@@ -121,7 +121,7 @@ void process_incoming_migration(QEMUFile *f)
     int fd = qemu_get_fd(f);
 
     assert(fd != -1);
-    socket_set_nonblock(fd);
+    qemu_set_nonblock(fd);
     qemu_coroutine_enter(co, f);
 }
 
diff --git a/nbd.c b/nbd.c
index d1a67ee..85187ff 100644
--- a/nbd.c
+++ b/nbd.c
@@ -386,7 +386,7 @@ static int nbd_send_negotiate(NBDClient *client)
         [28 .. 151]   reserved     (0)
      */
 
-    socket_set_block(csock);
+    qemu_set_block(csock);
     rc = -EINVAL;
 
     TRACE("Beginning negotiation.");
@@ -429,7 +429,7 @@ static int nbd_send_negotiate(NBDClient *client)
     TRACE("Negotiation succeeded.");
     rc = 0;
 fail:
-    socket_set_nonblock(csock);
+    qemu_set_nonblock(csock);
     return rc;
 }
 
@@ -443,7 +443,7 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
 
     TRACE("Receiving negotiation.");
 
-    socket_set_block(csock);
+    qemu_set_block(csock);
     rc = -EINVAL;
 
     if (read_sync(csock, buf, 8) != 8) {
@@ -558,7 +558,7 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
     rc = 0;
 
 fail:
-    socket_set_nonblock(csock);
+    qemu_set_nonblock(csock);
     return rc;
 }
 
diff --git a/net/socket.c b/net/socket.c
index 6c3752b..b5c8e65 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -308,7 +308,7 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr
         }
     }
 
-    socket_set_nonblock(fd);
+    qemu_set_nonblock(fd);
     return fd;
 fail:
     if (fd >= 0)
@@ -519,7 +519,7 @@ static int net_socket_listen_init(NetClientState *peer,
         perror("socket");
         return -1;
     }
-    socket_set_nonblock(fd);
+    qemu_set_nonblock(fd);
 
     /* allow fast reuse */
     val = 1;
@@ -565,7 +565,7 @@ static int net_socket_connect_init(NetClientState *peer,
         perror("socket");
         return -1;
     }
-    socket_set_nonblock(fd);
+    qemu_set_nonblock(fd);
 
     connected = 0;
     for(;;) {
diff --git a/qemu-char.c b/qemu-char.c
index 936150f..500a582 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2564,7 +2564,7 @@ static int tcp_chr_add_client(CharDriverState *chr, int fd)
     if (s->fd != -1)
 	return -1;
 
-    socket_set_nonblock(fd);
+    qemu_set_nonblock(fd);
     if (s->do_nodelay)
         socket_set_nodelay(fd);
     s->fd = fd;
@@ -2716,7 +2716,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
         printf("QEMU waiting for connection on: %s\n",
                chr->filename);
         tcp_chr_accept(s->listen_chan, G_IO_IN, chr);
-        socket_set_nonblock(s->listen_fd);
+        qemu_set_nonblock(s->listen_fd);
     }
     return chr;
 }
@@ -2758,7 +2758,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     }
 
     if (!is_waitconnect)
-        socket_set_nonblock(fd);
+        qemu_set_nonblock(fd);
 
     chr = qemu_chr_open_socket_fd(fd, do_nodelay, is_listen, is_telnet,
                                   is_waitconnect, &local_err);
@@ -3650,7 +3650,7 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
     if (error_is_set(errp)) {
         return NULL;
     }
-    socket_set_nonblock(fd);
+    qemu_set_nonblock(fd);
     return qemu_chr_open_tty_fd(fd);
 #else
     error_setg(errp, "character device backend type 'serial' not supported");
diff --git a/savevm.c b/savevm.c
index 406caa9..b1d8988 100644
--- a/savevm.c
+++ b/savevm.c
@@ -422,7 +422,7 @@ QEMUFile *qemu_fopen_socket(int fd, const char *mode)
 
     s->fd = fd;
     if (mode[0] == 'w') {
-        socket_set_block(s->fd);
+        qemu_set_block(s->fd);
         s->file = qemu_fopen_ops(s, &socket_write_ops);
     } else {
         s->file = qemu_fopen_ops(s, &socket_read_ops);
diff --git a/slirp/misc.c b/slirp/misc.c
index 6b9c2c4..8ecced5 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -215,7 +215,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
                 qemu_setsockopt(so->s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(int));
                 opt = 1;
                 qemu_setsockopt(so->s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int));
-		socket_set_nonblock(so->s);
+		qemu_set_nonblock(so->s);
 
 		/* Append the telnet options now */
                 if (so->so_m != NULL && do_pty == 1)  {
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 84a6bb5..e98ce1a 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -336,7 +336,7 @@ int tcp_fconnect(struct socket *so)
     int opt, s=so->s;
     struct sockaddr_in addr;
 
-    socket_set_nonblock(s);
+    qemu_set_nonblock(s);
     opt = 1;
     qemu_setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
     opt = 1;
@@ -425,7 +425,7 @@ void tcp_connect(struct socket *inso)
         tcp_close(sototcpcb(so)); /* This will sofree() as well */
         return;
     }
-    socket_set_nonblock(s);
+    qemu_set_nonblock(s);
     opt = 1;
     qemu_setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(int));
     opt = 1;
diff --git a/ui/vnc.c b/ui/vnc.c
index bbe1e0f..5ddb696 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2732,7 +2732,7 @@ static void vnc_connect(VncDisplay *vd, int csock, int skipauth, bool websocket)
 
     VNC_DEBUG("New client on socket %d\n", csock);
     vd->dcl.idle = 0;
-    socket_set_nonblock(vs->csock);
+    qemu_set_nonblock(vs->csock);
 #ifdef CONFIG_VNC_WS
     if (websocket) {
         vs->websocket = 1;
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 433dd68..4e4b819 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -134,14 +134,14 @@ void qemu_vfree(void *ptr)
     free(ptr);
 }
 
-void socket_set_block(int fd)
+void qemu_set_block(int fd)
 {
     int f;
     f = fcntl(fd, F_GETFL);
     fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
 }
 
-void socket_set_nonblock(int fd)
+void qemu_set_nonblock(int fd)
 {
     int f;
     f = fcntl(fd, F_GETFL);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 640194c..dcfa0c2 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -100,14 +100,14 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
     return p;
 }
 
-void socket_set_block(int fd)
+void qemu_set_block(int fd)
 {
     unsigned long opt = 0;
     WSAEventSelect(fd, NULL, 0);
     ioctlsocket(fd, FIONBIO, &opt);
 }
 
-void socket_set_nonblock(int fd)
+void qemu_set_nonblock(int fd)
 {
     unsigned long opt = 1;
     ioctlsocket(fd, FIONBIO, &opt);
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index b6b78f5..b632a74 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -277,7 +277,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
     }
     qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
     if (connect_state != NULL) {
-        socket_set_nonblock(sock);
+        qemu_set_nonblock(sock);
     }
     /* connect to peer */
     do {
@@ -737,7 +737,7 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
         connect_state = g_malloc0(sizeof(*connect_state));
         connect_state->callback = callback;
         connect_state->opaque = opaque;
-        socket_set_nonblock(sock);
+        qemu_set_nonblock(sock);
     }
 
     memset(&un, 0, sizeof(un));
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 2/4] net: ensure "socket" backend uses non-blocking fds

[Stefan Hajnoczi]

There are several code paths in net_init_socket() depending on how the
socket is created: file descriptor passing, UDP multicast, TCP, or UDP.
Some of these support both listen and connect.

Not all code paths set the socket to non-blocking.  This patch addresses
the file descriptor passing and UDP cases which were missing
socket_set_nonblock(fd) calls.

I considered moving socket_set_nonblock(fd) to a central location but it
turns out the code paths are different enough to require non-blocking at
different places.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/socket.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/socket.c b/net/socket.c
index b5c8e65..87af1d3 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -674,6 +674,7 @@ static int net_socket_udp_init(NetClientState *peer,
         closesocket(fd);
         return -1;
     }
+    qemu_set_nonblock(fd);
 
     s = net_socket_fd_init(peer, model, name, fd, 0);
     if (!s) {
@@ -712,7 +713,11 @@ int net_init_socket(const NetClientOptions *opts, const char *name,
         int fd;
 
         fd = monitor_handle_fd_param(cur_mon, sock->fd);
-        if (fd == -1 || !net_socket_fd_init(peer, "socket", name, fd, 1)) {
+        if (fd == -1) {
+            return -1;
+        }
+        qemu_set_nonblock(fd);
+        if (!net_socket_fd_init(peer, "socket", name, fd, 1)) {
             return -1;
         }
         return 0;
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 3/4] qemu-socket: set passed fd non-blocking in socket_connect()

[Stefan Hajnoczi]

socket_connect() sets non-blocking on TCP or UNIX domain sockets if a
callback function is passed.  Do the same for file descriptor passing,
otherwise we could unexpectedly be using a blocking file descriptor.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-sockets.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index b632a74..94581aa 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -910,6 +910,7 @@ int socket_connect(SocketAddress *addr, Error **errp,
     case SOCKET_ADDRESS_KIND_FD:
         fd = monitor_get_fd(cur_mon, addr->fd->str, errp);
         if (callback) {
+            qemu_set_nonblock(fd);
             callback(fd, opaque);
         }
         break;
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 4/4] chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors

[Stefan Hajnoczi]

When we receive a file descriptor over a UNIX domain socket the
O_NONBLOCK flag is preserved.  Clear the O_NONBLOCK flag and rely on
QEMU file descriptor users like migration, SPICE, VNC, block layer, and
others to set non-blocking only when necessary.

This change ensures we don't accidentally expose O_NONBLOCK in the QMP
API.  QMP clients should not need to get the non-blocking state
"correct".

A recent real-world example was when libvirt passed a non-blocking TCP
socket for migration where we expected a blocking socket.  The source
QEMU produced a corrupted migration stream since its code did not cope
with non-blocking sockets.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-char.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index 500a582..091f2dc 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2434,6 +2434,9 @@ static void unix_process_msgfd(CharDriverState *chr, struct msghdr *msg)
         if (fd < 0)
             continue;
 
+        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
+        qemu_set_block(fd);
+
 #ifndef MSG_CMSG_CLOEXEC
         qemu_set_cloexec(fd);
 #endif
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]

On 03/27/2013 03:10 AM, Stefan Hajnoczi wrote:
> There are several places where QEMU accidentally relies on the O_NONBLOCK state
> of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the QMP
> API whenever getfd or fdset_add_fd are used!
> 
> Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
> be hidden from QMP clients.
> 
> This patch series addresses this in 3 steps:
> 
> 1. Fix callers of monitor_handle_fd_param(), monitor_fdset_get_fd(), and
>    monitor_get_fd() that depend on O_NONBLOCK being set.  Luckily there are
>    only two instances and they are fixed in Patches 1 & 2.

Description is now off after rebase, but the cover letter isn't
committed, so no big deal.

> 
> 2. Rename socket_set_nonblock() to qemu_set_nonblock() just like
>    qemu_set_cloexec().  This makes code cleaner when working with arbitrary
>    file descriptors that may not be sockets.  See Patch 3.
> 
> 3. Clear O_NONBLOCK when a chardev receives file descriptors.  From now on QEMU
>    can assume that passed file descriptors are in blocking mode.  Simply use
>    qemu_set_nonblock(fd) if you want to enable O_NONBLOCK.  See Patch 4.

Series: Reviewed-by: Eric Blake <eblake@redhat.com>

> v2:
>  * Rename socket_set_nonblock() in Patch 1 to avoid code churn [eblake]
>  * Avoid qemu_set_block(-1) calls that clobber errno [quintela]
> 
> Stefan Hajnoczi (4):
>   oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()
>   net: ensure "socket" backend uses non-blocking fds
>   qemu-socket: set passed fd non-blocking in socket_connect()
>   chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

[Luiz Capitulino]

On Wed, 27 Mar 2013 10:10:42 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> There are several places where QEMU accidentally relies on the O_NONBLOCK state
> of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the QMP
> API whenever getfd or fdset_add_fd are used!
> 
> Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
> be hidden from QMP clients.
> 
> This patch series addresses this in 3 steps:

Nice series:

Applied to the qmp branch, thanks.

> 
> 1. Fix callers of monitor_handle_fd_param(), monitor_fdset_get_fd(), and
>    monitor_get_fd() that depend on O_NONBLOCK being set.  Luckily there are
>    only two instances and they are fixed in Patches 1 & 2.
> 
> 2. Rename socket_set_nonblock() to qemu_set_nonblock() just like
>    qemu_set_cloexec().  This makes code cleaner when working with arbitrary
>    file descriptors that may not be sockets.  See Patch 3.
> 
> 3. Clear O_NONBLOCK when a chardev receives file descriptors.  From now on QEMU
>    can assume that passed file descriptors are in blocking mode.  Simply use
>    qemu_set_nonblock(fd) if you want to enable O_NONBLOCK.  See Patch 4.
> 
> This fixes live migration with recent libvirt.  Libvirt checks if QEMU supports
> file descriptor passing and, if yes, hands QEMU a socket with O_NONBLOCK set.
> The migrate fd:<foo> code assumes the socket is in blocking mode.  The result
> is a corrupted migration stream.  For more info on this bug, see:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=923124
> 
> Note that Michal Privoznik <mprivozn@redhat.com> also sent a libvirt patch so
> that old QEMUs work with new libvirts:
> 
> https://www.redhat.com/archives/libvir-list/2013-March/msg01486.html
> 
> My patch series fixes the QMP API and allows old libvirts to work again with
> new QEMUs.
> 
> v2:
>  * Rename socket_set_nonblock() in Patch 1 to avoid code churn [eblake]
>  * Avoid qemu_set_block(-1) calls that clobber errno [quintela]
> 
> Stefan Hajnoczi (4):
>   oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()
>   net: ensure "socket" backend uses non-blocking fds
>   qemu-socket: set passed fd non-blocking in socket_connect()
>   chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors
> 
>  block/nbd.c            |  2 +-
>  block/sheepdog.c       |  2 +-
>  include/qemu/sockets.h |  4 ++--
>  migration.c            |  2 +-
>  nbd.c                  |  8 ++++----
>  net/socket.c           | 13 +++++++++----
>  qemu-char.c            | 11 +++++++----
>  savevm.c               |  2 +-
>  slirp/misc.c           |  2 +-
>  slirp/tcp_subr.c       |  4 ++--
>  ui/vnc.c               |  2 +-
>  util/oslib-posix.c     |  4 ++--
>  util/oslib-win32.c     |  4 ++--
>  util/qemu-sockets.c    |  5 +++--
>  14 files changed, 37 insertions(+), 28 deletions(-)
> 

Re: [Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

[mdroth]

On Wed, Mar 27, 2013 at 09:17:30AM -0400, Luiz Capitulino wrote:
> On Wed, 27 Mar 2013 10:10:42 +0100
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> > There are several places where QEMU accidentally relies on the O_NONBLOCK state
> > of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the QMP
> > API whenever getfd or fdset_add_fd are used!
> > 
> > Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
> > be hidden from QMP clients.
> > 
> > This patch series addresses this in 3 steps:
> 
> Nice series:
> 
> Applied to the qmp branch, thanks.

Hi Luiz,

Eric/mjt have noted that this series fixes a number of issues in 1.4.0
and have requested it for 1.4.1

http://thread.gmane.org/gmane.comp.emulators.qemu/203851

The cut-off for 1.4.1 is Tuesday. Do you plan to send a pull for these soon?

> 
> > 
> > 1. Fix callers of monitor_handle_fd_param(), monitor_fdset_get_fd(), and
> >    monitor_get_fd() that depend on O_NONBLOCK being set.  Luckily there are
> >    only two instances and they are fixed in Patches 1 & 2.
> > 
> > 2. Rename socket_set_nonblock() to qemu_set_nonblock() just like
> >    qemu_set_cloexec().  This makes code cleaner when working with arbitrary
> >    file descriptors that may not be sockets.  See Patch 3.
> > 
> > 3. Clear O_NONBLOCK when a chardev receives file descriptors.  From now on QEMU
> >    can assume that passed file descriptors are in blocking mode.  Simply use
> >    qemu_set_nonblock(fd) if you want to enable O_NONBLOCK.  See Patch 4.
> > 
> > This fixes live migration with recent libvirt.  Libvirt checks if QEMU supports
> > file descriptor passing and, if yes, hands QEMU a socket with O_NONBLOCK set.
> > The migrate fd:<foo> code assumes the socket is in blocking mode.  The result
> > is a corrupted migration stream.  For more info on this bug, see:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=923124
> > 
> > Note that Michal Privoznik <mprivozn@redhat.com> also sent a libvirt patch so
> > that old QEMUs work with new libvirts:
> > 
> > https://www.redhat.com/archives/libvir-list/2013-March/msg01486.html
> > 
> > My patch series fixes the QMP API and allows old libvirts to work again with
> > new QEMUs.
> > 
> > v2:
> >  * Rename socket_set_nonblock() in Patch 1 to avoid code churn [eblake]
> >  * Avoid qemu_set_block(-1) calls that clobber errno [quintela]
> > 
> > Stefan Hajnoczi (4):
> >   oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()
> >   net: ensure "socket" backend uses non-blocking fds
> >   qemu-socket: set passed fd non-blocking in socket_connect()
> >   chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors
> > 
> >  block/nbd.c            |  2 +-
> >  block/sheepdog.c       |  2 +-
> >  include/qemu/sockets.h |  4 ++--
> >  migration.c            |  2 +-
> >  nbd.c                  |  8 ++++----
> >  net/socket.c           | 13 +++++++++----
> >  qemu-char.c            | 11 +++++++----
> >  savevm.c               |  2 +-
> >  slirp/misc.c           |  2 +-
> >  slirp/tcp_subr.c       |  4 ++--
> >  ui/vnc.c               |  2 +-
> >  util/oslib-posix.c     |  4 ++--
> >  util/oslib-win32.c     |  4 ++--
> >  util/qemu-sockets.c    |  5 +++--
> >  14 files changed, 37 insertions(+), 28 deletions(-)
> > 
> 
> 

Re: [Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

[Luiz Capitulino]

On Wed, 3 Apr 2013 10:05:40 -0500
mdroth <mdroth@linux.vnet.ibm.com> wrote:

> On Wed, Mar 27, 2013 at 09:17:30AM -0400, Luiz Capitulino wrote:
> > On Wed, 27 Mar 2013 10:10:42 +0100
> > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > 
> > > There are several places where QEMU accidentally relies on the O_NONBLOCK state
> > > of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the QMP
> > > API whenever getfd or fdset_add_fd are used!
> > > 
> > > Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
> > > be hidden from QMP clients.
> > > 
> > > This patch series addresses this in 3 steps:
> > 
> > Nice series:
> > 
> > Applied to the qmp branch, thanks.
> 
> Hi Luiz,
> 
> Eric/mjt have noted that this series fixes a number of issues in 1.4.0
> and have requested it for 1.4.1
> 
> http://thread.gmane.org/gmane.comp.emulators.qemu/203851
> 
> The cut-off for 1.4.1 is Tuesday. Do you plan to send a pull for these soon?

Sure, I can do that. And thanks to remind me.

Re: [Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

[Luiz Capitulino]

On Wed, 3 Apr 2013 11:24:38 -0400
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> On Wed, 3 Apr 2013 10:05:40 -0500
> mdroth <mdroth@linux.vnet.ibm.com> wrote:
> 
> > On Wed, Mar 27, 2013 at 09:17:30AM -0400, Luiz Capitulino wrote:
> > > On Wed, 27 Mar 2013 10:10:42 +0100
> > > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > > 
> > > > There are several places where QEMU accidentally relies on the O_NONBLOCK state
> > > > of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the QMP
> > > > API whenever getfd or fdset_add_fd are used!
> > > > 
> > > > Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
> > > > be hidden from QMP clients.
> > > > 
> > > > This patch series addresses this in 3 steps:
> > > 
> > > Nice series:
> > > 
> > > Applied to the qmp branch, thanks.
> > 
> > Hi Luiz,
> > 
> > Eric/mjt have noted that this series fixes a number of issues in 1.4.0
> > and have requested it for 1.4.1
> > 
> > http://thread.gmane.org/gmane.comp.emulators.qemu/203851
> > 
> > The cut-off for 1.4.1 is Tuesday. Do you plan to send a pull for these soon?
> 
> Sure, I can do that. And thanks to remind me.

Actually, I can't cherry-pick it cleanly.

Stefan, would you mind to backport this series to v1.4.1 yourself?