[Qemu-devel] [PATCH 0/5 v2] Provide common methods for exchange FD

[Qemu-devel] [PATCH 0/5 v2] Provide common methods for exchange FD

[Lei Li]

This patch series tries to refactor the functions used for 
exchange of FD in current code, provide common methods
for it.

I just tested it through page flipping migration, and tap/
bridge-helper a bit, but have some environment problem on
proxy fs driver. So it'd be appreciated if someone could
help on verifying whether it has impact on it. :)

Please let me know if there is anything needs to be improved.

Thanks


Changes since V1:
  -- Copy right and typo fixes pointed out by Eric.
  -- Don't cast 'char *' to 'int *' from Daniel.
  -- Get rid of local migration part.

Lei Li (5):
  fd-exchange: provide common methods for exchange of fd
  qemu-bridge-helper: replace send_fd with qemu_send_with_fd
  net/tap: replace recv_fd with qemu_recv_with_fd
  virtfs-proxy-helper: replace send_fd with qemu_send_with_fd
  virtio-9p-proxy: replace v9fs_receivefd with qemu_recv_with_fd

 Makefile                    |    2 +-
 fsdev/virtfs-proxy-helper.c |   51 ++++-------------------
 hw/9pfs/virtio-9p-proxy.c   |   60 +-------------------------
 hw/9pfs/virtio-9p-proxy.h   |    5 --
 include/qemu/fd-exchange.h  |   25 +++++++++++
 net/tap.c                   |   40 +----------------
 qemu-bridge-helper.c        |   31 +------------
 util/Makefile.objs          |    1 +
 util/qemu-fd-exchange.c     |   97 +++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 144 insertions(+), 220 deletions(-)
 create mode 100644 include/qemu/fd-exchange.h
 create mode 100644 util/qemu-fd-exchange.c

[Qemu-devel] [PATCH 1/5] qemu-fd-exchange: provide common methods for exchange fd

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 include/qemu/fd-exchange.h |   25 +++++++++++
 util/Makefile.objs         |    1 +
 util/qemu-fd-exchange.c    |   97 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 0 deletions(-)
 create mode 100644 include/qemu/fd-exchange.h
 create mode 100644 util/qemu-fd-exchange.c

diff --git a/include/qemu/fd-exchange.h b/include/qemu/fd-exchange.h
new file mode 100644
index 0000000..8502960
--- /dev/null
+++ b/include/qemu/fd-exchange.h
@@ -0,0 +1,25 @@
+/*
+ * Internal common methods for exchange of FD
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef FD_EXCHANGE_H
+#define FD_EXCHANGE_H
+
+#include <sys/socket.h>
+
+union MsgControl {
+    struct cmsghdr cmsg;
+    char control[CMSG_SPACE(sizeof(int))];
+};
+
+ssize_t qemu_send_with_fd(int sockfd, int passed_fd,
+                          const void *buf, size_t len);
+
+ssize_t qemu_recv_with_fd(int sockfd, int *passed_fd,
+                          void *buf, size_t len);
+
+#endif
diff --git a/util/Makefile.objs b/util/Makefile.objs
index af3e5cb..2fb42bf 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -13,3 +13,4 @@ util-obj-y += hexdump.o
 util-obj-y += crc32c.o
 util-obj-y += throttle.o
 util-obj-y += getauxval.o
+util-obj-y += qemu-fd-exchange.o
diff --git a/util/qemu-fd-exchange.c b/util/qemu-fd-exchange.c
new file mode 100644
index 0000000..bee3fc1
--- /dev/null
+++ b/util/qemu-fd-exchange.c
@@ -0,0 +1,97 @@
+/*
+ * Internal common methods for exchange of FD
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/fd-exchange.h"
+#include "qemu-common.h"
+
+
+ssize_t qemu_send_with_fd(int sockfd, int passed_fd,
+                          const void *buf, size_t len)
+{
+    struct msghdr msg;
+    struct iovec iov;
+    struct cmsghdr *cmsg;
+    union MsgControl msg_control;
+    int retval;
+
+    iov.iov_base = (char *)buf;
+    iov.iov_len = len;
+
+    memset(&msg, 0, sizeof(msg));
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = len;
+    msg.msg_control = &msg_control;
+    msg.msg_controllen = sizeof(msg_control);
+
+    if (passed_fd < 0) {
+        *(char *)buf = passed_fd;
+    } else {
+        msg.msg_control = &msg_control;
+        msg.msg_controllen = sizeof(msg_control);
+
+        cmsg = &msg_control.cmsg;
+        cmsg->cmsg_len = CMSG_LEN(sizeof(passed_fd));
+        cmsg->cmsg_level = SOL_SOCKET;
+        cmsg->cmsg_type = SCM_RIGHTS;
+        memcpy(CMSG_DATA(cmsg), &passed_fd, sizeof(passed_fd));
+
+    }
+
+    do {
+        retval = sendmsg(sockfd, &msg, 0);
+    } while (retval < 0 && errno == EINTR);
+
+    return retval;
+}
+
+ssize_t qemu_recv_with_fd(int sockfd, int *passed_fd,
+                          void *buf, size_t len)
+{
+    struct iovec iov;
+    struct msghdr msg;
+    struct cmsghdr *cmsg;
+    union MsgControl msg_control;
+    int retval;
+    char data;
+
+    iov.iov_base = &data;
+    iov.iov_len = len;
+
+    memset(&msg, 0, sizeof(msg));
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
+    msg.msg_control = &msg_control;
+    msg.msg_controllen = sizeof(msg_control);
+
+    do {
+        retval = recvmsg(sockfd, &msg, 0);
+    } while (retval < 0 && errno == EINTR);
+
+    if (retval <= 0) {
+        return retval;
+    }
+
+    if (data != *(char *)buf) {
+        *passed_fd = data;
+        return 0;
+    }
+
+    for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
+        if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) ||
+            cmsg->cmsg_level != SOL_SOCKET ||
+            cmsg->cmsg_type != SCM_RIGHTS) {
+            continue;
+        }
+
+        memcpy(passed_fd, CMSG_DATA(cmsg), sizeof(*passed_fd));
+        return 0;
+    }
+
+    *passed_fd = -ENFILE;
+    return retval;
+}
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/5] qemu-bridge-helper: replace send_fd with qemu_send_with_fd

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 Makefile             |    2 +-
 qemu-bridge-helper.c |   31 +++----------------------------
 2 files changed, 4 insertions(+), 29 deletions(-)

diff --git a/Makefile b/Makefile
index bdff4e4..6850f35 100644
--- a/Makefile
+++ b/Makefile
@@ -195,7 +195,7 @@ qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a
 qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a
 qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a
 
-qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
+qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o libqemuutil.a
 
 fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o libqemuutil.a libqemustub.a
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 6a0974e..8303b6b 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -40,6 +40,7 @@
 #endif
 
 #include "qemu/queue.h"
+#include "qemu/fd-exchange.h"
 
 #include "net/tap-linux.h"
 
@@ -174,33 +175,6 @@ static void prep_ifreq(struct ifreq *ifr, const char *ifname)
     snprintf(ifr->ifr_name, IFNAMSIZ, "%s", ifname);
 }
 
-static int send_fd(int c, int fd)
-{
-    char msgbuf[CMSG_SPACE(sizeof(fd))];
-    struct msghdr msg = {
-        .msg_control = msgbuf,
-        .msg_controllen = sizeof(msgbuf),
-    };
-    struct cmsghdr *cmsg;
-    struct iovec iov;
-    char req[1] = { 0x00 };
-
-    cmsg = CMSG_FIRSTHDR(&msg);
-    cmsg->cmsg_level = SOL_SOCKET;
-    cmsg->cmsg_type = SCM_RIGHTS;
-    cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
-    msg.msg_controllen = cmsg->cmsg_len;
-
-    iov.iov_base = req;
-    iov.iov_len = sizeof(req);
-
-    msg.msg_iov = &iov;
-    msg.msg_iovlen = 1;
-    memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
-
-    return sendmsg(c, &msg, 0);
-}
-
 #ifdef CONFIG_LIBCAP
 static int drop_privileges(void)
 {
@@ -239,6 +213,7 @@ int main(int argc, char **argv)
     ACLList acl_list;
     int access_allowed, access_denied;
     int ret = EXIT_SUCCESS;
+    char req[1] = { 0x00 };
 
 #ifdef CONFIG_LIBCAP
     /* if we're run from an suid binary, immediately drop privileges preserving
@@ -424,7 +399,7 @@ int main(int argc, char **argv)
     }
 
     /* write fd to the domain socket */
-    if (send_fd(unixfd, fd) == -1) {
+    if (qemu_send_with_fd(unixfd, fd, &req, sizeof(req)) == -1) {
         fprintf(stderr, "failed to write fd to unix socket: %s\n",
                 strerror(errno));
         ret = EXIT_FAILURE;
-- 
1.7.7.6

[Qemu-devel] [PATCH 3/5] net/tap: replace recv_fd with qemu_recv_with_fd

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 net/tap.c |   40 +++-------------------------------------
 1 files changed, 3 insertions(+), 37 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 39c1cda..97ee2e8 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -39,6 +39,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu-common.h"
 #include "qemu/error-report.h"
+#include "qemu/fd-exchange.h"
 
 #include "net/tap.h"
 
@@ -385,40 +386,6 @@ static int launch_script(const char *setup_script, const char *ifname, int fd)
     return -1;
 }
 
-static int recv_fd(int c)
-{
-    int fd;
-    uint8_t msgbuf[CMSG_SPACE(sizeof(fd))];
-    struct msghdr msg = {
-        .msg_control = msgbuf,
-        .msg_controllen = sizeof(msgbuf),
-    };
-    struct cmsghdr *cmsg;
-    struct iovec iov;
-    uint8_t req[1];
-    ssize_t len;
-
-    cmsg = CMSG_FIRSTHDR(&msg);
-    cmsg->cmsg_level = SOL_SOCKET;
-    cmsg->cmsg_type = SCM_RIGHTS;
-    cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
-    msg.msg_controllen = cmsg->cmsg_len;
-
-    iov.iov_base = req;
-    iov.iov_len = sizeof(req);
-
-    msg.msg_iov = &iov;
-    msg.msg_iovlen = 1;
-
-    len = recvmsg(c, &msg, 0);
-    if (len > 0) {
-        memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd));
-        return fd;
-    }
-
-    return len;
-}
-
 static int net_bridge_run_helper(const char *helper, const char *bridge)
 {
     sigset_t oldmask, mask;
@@ -489,12 +456,11 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
 
     } else if (pid > 0) {
         int fd;
+        char req[1] = { 0x00 };
 
         close(sv[1]);
 
-        do {
-            fd = recv_fd(sv[0]);
-        } while (fd == -1 && errno == EINTR);
+        qemu_recv_with_fd(sv[0], &fd, &req, sizeof(req));
 
         close(sv[0]);
 
-- 
1.7.7.6

[Qemu-devel] [PATCH 4/5] virtfs-proxy-helper: replace send_fd with qemu_send_with_fd

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 fsdev/virtfs-proxy-helper.c |   51 ++++++------------------------------------
 hw/9pfs/virtio-9p-proxy.h   |    5 ----
 2 files changed, 8 insertions(+), 48 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 713a7b2..44c6e61 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -23,6 +23,7 @@
 #include "qemu-common.h"
 #include "qemu/sockets.h"
 #include "qemu/xattr.h"
+#include "qemu/fd-exchange.h"
 #include "virtio-9p-marshal.h"
 #include "hw/9pfs/virtio-9p-proxy.h"
 #include "fsdev/virtio-9p-marshal.h"
@@ -203,48 +204,6 @@ static int read_request(int sockfd, struct iovec *iovec, ProxyHeader *header)
     return 0;
 }
 
-static int send_fd(int sockfd, int fd)
-{
-    struct msghdr msg;
-    struct iovec iov;
-    int retval, data;
-    struct cmsghdr *cmsg;
-    union MsgControl msg_control;
-
-    iov.iov_base = &data;
-    iov.iov_len = sizeof(data);
-
-    memset(&msg, 0, sizeof(msg));
-    msg.msg_iov = &iov;
-    msg.msg_iovlen = 1;
-    /* No ancillary data on error */
-    if (fd < 0) {
-        /* fd is really negative errno if the request failed  */
-        data = fd;
-    } else {
-        data = V9FS_FD_VALID;
-        msg.msg_control = &msg_control;
-        msg.msg_controllen = sizeof(msg_control);
-
-        cmsg = &msg_control.cmsg;
-        cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
-        cmsg->cmsg_level = SOL_SOCKET;
-        cmsg->cmsg_type = SCM_RIGHTS;
-        memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
-    }
-
-    do {
-        retval = sendmsg(sockfd, &msg, 0);
-    } while (retval < 0 && errno == EINTR);
-    if (fd >= 0) {
-        close(fd);
-    }
-    if (retval < 0) {
-        return retval;
-    }
-    return 0;
-}
-
 static int send_status(int sockfd, struct iovec *iovec, int status)
 {
     ProxyHeader header;
@@ -784,11 +743,17 @@ static void usage(char *prog)
 static int process_reply(int sock, int type,
                          struct iovec *out_iovec, int retval)
 {
+    int data = V9FS_FD_VALID;
+
     switch (type) {
     case T_OPEN:
     case T_CREATE:
-        if (send_fd(sock, retval) < 0) {
+        if (qemu_send_with_fd(sock, retval, &data, sizeof(data)) < 0) {
             return -1;
+        } else {
+            if (retval >= 0) {
+                close(retval);
+            }
         }
         break;
     case T_MKNOD:
diff --git a/hw/9pfs/virtio-9p-proxy.h b/hw/9pfs/virtio-9p-proxy.h
index 005c1ad..e359ac5 100644
--- a/hw/9pfs/virtio-9p-proxy.h
+++ b/hw/9pfs/virtio-9p-proxy.h
@@ -24,11 +24,6 @@
 #define proxy_marshal(out_sg, offset, fmt, args...) \
     v9fs_marshal(out_sg, 1, offset, 0, fmt, ##args)
 
-union MsgControl {
-    struct cmsghdr cmsg;
-    char control[CMSG_SPACE(sizeof(int))];
-};
-
 typedef struct {
     uint32_t type;
     uint32_t size;
-- 
1.7.7.6

[Qemu-devel] [PATCH 5/5] virtio-9p-proxy: replace v9fs_receivefd with qemu_recv_with_fd

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p-proxy.c |   60 ++------------------------------------------
 1 files changed, 3 insertions(+), 57 deletions(-)

diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index 5f44bb7..f34b845 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -14,6 +14,7 @@
 #include "hw/virtio/virtio.h"
 #include "virtio-9p.h"
 #include "qemu/error-report.h"
+#include "qemu/fd-exchange.h"
 #include "fsdev/qemu-fsdev.h"
 #include "virtio-9p-proxy.h"
 
@@ -24,62 +25,6 @@ typedef struct V9fsProxy {
     struct iovec out_iovec;
 } V9fsProxy;
 
-/*
- * Return received file descriptor on success in *status.
- * errno is also returned on *status (which will be < 0)
- * return < 0 on transport error.
- */
-static int v9fs_receivefd(int sockfd, int *status)
-{
-    struct iovec iov;
-    struct msghdr msg;
-    struct cmsghdr *cmsg;
-    int retval, data, fd;
-    union MsgControl msg_control;
-
-    iov.iov_base = &data;
-    iov.iov_len = sizeof(data);
-
-    memset(&msg, 0, sizeof(msg));
-    msg.msg_iov = &iov;
-    msg.msg_iovlen = 1;
-    msg.msg_control = &msg_control;
-    msg.msg_controllen = sizeof(msg_control);
-
-    do {
-        retval = recvmsg(sockfd, &msg, 0);
-    } while (retval < 0 && errno == EINTR);
-    if (retval <= 0) {
-        return retval;
-    }
-    /*
-     * data is set to V9FS_FD_VALID, if ancillary data is sent.  If this
-     * request doesn't need ancillary data (fd) or an error occurred,
-     * data is set to negative errno value.
-     */
-    if (data != V9FS_FD_VALID) {
-        *status = data;
-        return 0;
-    }
-    /*
-     * File descriptor (fd) is sent in the ancillary data. Check if we
-     * indeed received it. One of the reasons to fail to receive it is if
-     * we exceeded the maximum number of file descriptors!
-     */
-    for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
-        if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) ||
-            cmsg->cmsg_level != SOL_SOCKET ||
-            cmsg->cmsg_type != SCM_RIGHTS) {
-            continue;
-        }
-        fd = *((int *)CMSG_DATA(cmsg));
-        *status = fd;
-        return 0;
-    }
-    *status = -ENFILE;  /* Ancillary data sent but not received */
-    return 0;
-}
-
 static ssize_t socket_read(int sockfd, void *buff, size_t size)
 {
     ssize_t retval, total = 0;
@@ -307,6 +252,7 @@ static int v9fs_request(V9fsProxy *proxy, int type,
     V9fsString *name, *value;
     V9fsString *path, *oldpath;
     struct iovec *iovec = NULL, *reply = NULL;
+    int data = V9FS_FD_VALID;
 
     qemu_mutex_lock(&proxy->mutex);
 
@@ -548,7 +494,7 @@ static int v9fs_request(V9fsProxy *proxy, int type,
          * A file descriptor is returned as response for
          * T_OPEN,T_CREATE on success
          */
-        if (v9fs_receivefd(proxy->sockfd, &retval) < 0) {
+        if (qemu_recv_with_fd(proxy->sockfd, &retval, &data, sizeof(data)) < 0) {
             goto close_error;
         }
         break;
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 0/5 v2] Provide common methods for exchange FD

[Antonios Motakis]

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

Hello,


On Thu, Jan 23, 2014 at 9:27 AM, Lei Li <lilei@linux.vnet.ibm.com> wrote:

> This patch series tries to refactor the functions used for
> exchange of FD in current code, provide common methods
> for it.
>
> I just tested it through page flipping migration, and tap/
> bridge-helper a bit, but have some environment problem on
> proxy fs driver. So it'd be appreciated if someone could
> help on verifying whether it has impact on it. :)
>
> Please let me know if there is anything needs to be improved.
>
> Thanks
>
>
This looks really interesting. We are currently working on a patch series
that will also need to exchange file descriptors(see vhost_user_send_fds in
http://lists.gnu.org/archive/html/qemu-devel/2014-01/msg01444.html).

I wonder if you have taken into account the case of sending multiple file
descriptors in one go.

Antonios


>
> Changes since V1:
>   -- Copy right and typo fixes pointed out by Eric.
>   -- Don't cast 'char *' to 'int *' from Daniel.
>   -- Get rid of local migration part.
>
> Lei Li (5):
>   fd-exchange: provide common methods for exchange of fd
>   qemu-bridge-helper: replace send_fd with qemu_send_with_fd
>   net/tap: replace recv_fd with qemu_recv_with_fd
>   virtfs-proxy-helper: replace send_fd with qemu_send_with_fd
>   virtio-9p-proxy: replace v9fs_receivefd with qemu_recv_with_fd
>
>  Makefile                    |    2 +-
>  fsdev/virtfs-proxy-helper.c |   51 ++++-------------------
>  hw/9pfs/virtio-9p-proxy.c   |   60 +-------------------------
>  hw/9pfs/virtio-9p-proxy.h   |    5 --
>  include/qemu/fd-exchange.h  |   25 +++++++++++
>  net/tap.c                   |   40 +----------------
>  qemu-bridge-helper.c        |   31 +------------
>  util/Makefile.objs          |    1 +
>  util/qemu-fd-exchange.c     |   97
> +++++++++++++++++++++++++++++++++++++++++++
>  10 files changed, 144 insertions(+), 220 deletions(-)
>  create mode 100644 include/qemu/fd-exchange.h
>  create mode 100644 util/qemu-fd-exchange.c
>
>
>

[-- Attachment #2: Type: text/html, Size: 2833 bytes --]