diff mbox series

[v8,06/20] multi-process: define MPQemuMsg format and transmission functions

Message ID 50f84ecac23889920aeed397adb9317b8ea5a54b.1596217462.git.jag.raman@oracle.com
State New
Headers show
Series Initial support for multi-process qemu | expand

Commit Message

Jag Raman July 31, 2020, 6:20 p.m. UTC
From: Elena Ufimtseva <elena.ufimtseva@oracle.com>

Defines MPQemuMsg, which is the message that is sent to the remote
process. This message is sent over QIOChannel and is used to
command the remote process to perform various tasks.

Also defined the helper functions to send and receive messages over the
QIOChannel

Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
---
 MAINTAINERS              |   2 +
 include/io/mpqemu-link.h |  73 ++++++++++++++++++++
 io/Makefile.objs         |   2 +
 io/mpqemu-link.c         | 175 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 252 insertions(+)
 create mode 100644 include/io/mpqemu-link.h
 create mode 100644 io/mpqemu-link.c

Comments

Stefan Hajnoczi Aug. 4, 2020, 12:49 p.m. UTC | #1
On Fri, Jul 31, 2020 at 02:20:13PM -0400, Jagannathan Raman wrote:
> +static int mpqemu_readv(QIOChannel *ioc, struct iovec *iov, int **fds,
> +                        size_t *nfds, Error **errp)

readv(2) and similar functions take an int iovcnt argument while
mpqemu_readv() takes just a single struct iovec. The name mpqemu_read()
seems more appopriate and iov argument could be replaced by void *buf,
size_t len. That is cleaner because this function modifies the iov
argument (callers need to be be careful!).

> +{
> +    struct iovec *l_iov = iov;
> +    size_t *l_nfds = nfds;
> +    unsigned int niov = 1;
> +    int **l_fds = fds;
> +    size_t size, len;
> +    Error *local_err = NULL;
> +
> +    size = iov->iov_len;
> +
> +    while (size > 0) {
> +        len = qio_channel_readv_full(ioc, l_iov, niov, l_fds, l_nfds,
> +                                     &local_err);
> +
> +        if (len == QIO_CHANNEL_ERR_BLOCK) {
> +            if (qemu_in_coroutine()) {
> +                qio_channel_yield(ioc, G_IO_IN);
> +            } else {
> +                qio_channel_wait(ioc, G_IO_IN);
> +            }
> +            continue;
> +        }
> +
> +        if (len <= 0) {

size_t is unsigned so this does not detect negative values. Please use
qio_channel_readv_full()'s return type (ssize_t) instead.

> +            error_propagate(errp, local_err);
> +            return -EIO;
> +        }
> +
> +        l_fds = NULL;
> +        l_nfds = NULL;
> +
> +        size -= len;
> +
> +        (void)iov_discard_front(&l_iov, &niov, len);
> +    }
> +
> +    return iov->iov_len;

read()-style functions return the number of bytes read. mpqemu_readv()
returns the number of unread bytes remaining. Maintaining consistency
with read() function conventions will reduce the chance of bugs, so I
suggest returning the number of bytes read instead.

> +}
> +
> +void mpqemu_msg_recv(MPQemuMsg *msg, QIOChannel *ioc, Error **errp)
> +{
> +    Error *local_err = NULL;
> +    int *fds = NULL;
> +    struct iovec hdr, data;
> +    size_t nfds = 0;
> +
> +    hdr.iov_base = msg;
> +    hdr.iov_len = MPQEMU_MSG_HDR_SIZE;
> +
> +    if (mpqemu_readv(ioc, &hdr, &fds, &nfds, &local_err) < 0) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }

Can we read less than MPQEMU_MSG_HDR_SIZE if the peer closes the socket?
That case probably needs to be handled.

> +void mpqemu_msg_cleanup(MPQemuMsg *msg)
> +{
> +    if (msg->data2) {
> +        free(msg->data2);

The buffer was allocated with g_malloc0() so g_free() should be used to
free it.
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 32e264e..41fe809 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3044,6 +3044,8 @@  F: hw/pci-host/remote.c
 F: include/hw/pci-host/remote.h
 F: hw/i386/remote.c
 F: include/hw/i386/remote.h
+F: io/mpqemu-link.c
+F: include/io/mpqemu-link.h
 
 Build and test automation
 -------------------------
diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
new file mode 100644
index 0000000..ae7008e
--- /dev/null
+++ b/include/io/mpqemu-link.h
@@ -0,0 +1,73 @@ 
+/*
+ * Communication channel between QEMU and remote device process
+ *
+ * Copyright © 2018, 2020 Oracle and/or its affiliates.
+ *
+ * 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 MPQEMU_LINK_H
+#define MPQEMU_LINK_H
+
+#include "qom/object.h"
+#include "qemu/thread.h"
+#include "io/channel.h"
+
+#define REMOTE_MAX_FDS 8
+
+#define MPQEMU_MSG_HDR_SIZE offsetof(MPQemuMsg, data1.u64)
+
+/**
+ * MPQemuCmd:
+ *
+ * MPQemuCmd enum type to specify the command to be executed on the remote
+ * device.
+ */
+typedef enum {
+    INIT = 0,
+    MAX = INT_MAX,
+} MPQemuCmd;
+
+/**
+ * Maximum size of data2 field in the message to be transmitted.
+ */
+#define MPQEMU_MSG_DATA_MAX 256
+
+/**
+ * MPQemuMsg:
+ * @cmd: The remote command
+ * @bytestream: Indicates if the data to be shared is structured (data1)
+ *              or unstructured (data2)
+ * @size: Size of the data to be shared
+ * @data1: Structured data
+ * @fds: File descriptors to be shared with remote device
+ * @data2: Unstructured data
+ *
+ * MPQemuMsg Format of the message sent to the remote device from QEMU.
+ *
+ */
+typedef struct {
+    int cmd;
+    int bytestream;
+    size_t size;
+
+    union {
+        uint64_t u64;
+    } data1;
+
+    int fds[REMOTE_MAX_FDS];
+    int num_fds;
+
+    /* Max size of data2 is MPQEMU_MSG_DATA_MAX */
+    uint8_t *data2;
+} MPQemuMsg;
+
+void mpqemu_msg_send(MPQemuMsg *msg, QIOChannel *ioc, Error **errp);
+void mpqemu_msg_recv(MPQemuMsg *msg, QIOChannel *ioc, Error **errp);
+
+bool mpqemu_msg_valid(MPQemuMsg *msg);
+void mpqemu_msg_cleanup(MPQemuMsg *msg);
+
+#endif
diff --git a/io/Makefile.objs b/io/Makefile.objs
index 9a20fce..5875ab0 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -10,3 +10,5 @@  io-obj-y += channel-util.o
 io-obj-y += dns-resolver.o
 io-obj-y += net-listener.o
 io-obj-y += task.o
+
+io-obj-$(CONFIG_MPQEMU) += mpqemu-link.o
diff --git a/io/mpqemu-link.c b/io/mpqemu-link.c
new file mode 100644
index 0000000..dcefa42
--- /dev/null
+++ b/io/mpqemu-link.c
@@ -0,0 +1,175 @@ 
+/*
+ * Communication channel between QEMU and remote device process
+ *
+ * Copyright © 2018, 2020 Oracle and/or its affiliates.
+ *
+ * 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/osdep.h"
+#include "qemu-common.h"
+
+#include "qemu/module.h"
+#include "io/mpqemu-link.h"
+#include "qapi/error.h"
+#include "qemu/iov.h"
+#include "qemu/error-report.h"
+
+void mpqemu_msg_send(MPQemuMsg *msg, QIOChannel *ioc, Error **errp)
+{
+    Error *local_err = NULL;
+    struct iovec send[2];
+    int *fds = NULL;
+    size_t nfds = 0;
+
+    send[0].iov_base = msg;
+    send[0].iov_len = MPQEMU_MSG_HDR_SIZE;
+
+    send[1].iov_base = msg->bytestream ? msg->data2 : (void *)&msg->data1;
+    send[1].iov_len = msg->size;
+
+    if (msg->num_fds) {
+        nfds = msg->num_fds;
+        fds = msg->fds;
+    }
+
+    (void)qio_channel_writev_full_all(ioc, send, G_N_ELEMENTS(send), fds, nfds,
+                                      &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+
+    return;
+}
+
+static int mpqemu_readv(QIOChannel *ioc, struct iovec *iov, int **fds,
+                        size_t *nfds, Error **errp)
+{
+    struct iovec *l_iov = iov;
+    size_t *l_nfds = nfds;
+    unsigned int niov = 1;
+    int **l_fds = fds;
+    size_t size, len;
+    Error *local_err = NULL;
+
+    size = iov->iov_len;
+
+    while (size > 0) {
+        len = qio_channel_readv_full(ioc, l_iov, niov, l_fds, l_nfds,
+                                     &local_err);
+
+        if (len == QIO_CHANNEL_ERR_BLOCK) {
+            if (qemu_in_coroutine()) {
+                qio_channel_yield(ioc, G_IO_IN);
+            } else {
+                qio_channel_wait(ioc, G_IO_IN);
+            }
+            continue;
+        }
+
+        if (len <= 0) {
+            error_propagate(errp, local_err);
+            return -EIO;
+        }
+
+        l_fds = NULL;
+        l_nfds = NULL;
+
+        size -= len;
+
+        (void)iov_discard_front(&l_iov, &niov, len);
+    }
+
+    return iov->iov_len;
+}
+
+void mpqemu_msg_recv(MPQemuMsg *msg, QIOChannel *ioc, Error **errp)
+{
+    Error *local_err = NULL;
+    int *fds = NULL;
+    struct iovec hdr, data;
+    size_t nfds = 0;
+
+    hdr.iov_base = msg;
+    hdr.iov_len = MPQEMU_MSG_HDR_SIZE;
+
+    if (mpqemu_readv(ioc, &hdr, &fds, &nfds, &local_err) < 0) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    if (msg->size > MPQEMU_MSG_DATA_MAX) {
+        error_setg(errp, "The message size is more than MPQEMU_MSG_DATA_MAX %d",
+                     MPQEMU_MSG_DATA_MAX);
+        return;
+    }
+
+    data.iov_base = g_malloc0(msg->size);
+    data.iov_len = msg->size;
+
+    if (mpqemu_readv(ioc, &data, NULL, NULL, &local_err) < 0) {
+        error_propagate(errp, local_err);
+        g_free(data.iov_base);
+        return;
+    }
+
+    if (msg->bytestream) {
+        msg->data2 = data.iov_base;
+        data.iov_base = NULL;
+    } else if (msg->size > sizeof(msg->data1)) {
+        error_setg(errp, "Invalid size for message");
+        g_free(data.iov_base);
+    } else {
+        memcpy((void *)&msg->data1, data.iov_base, msg->size);
+        g_free(data.iov_base);
+    }
+
+    msg->num_fds = nfds;
+    if (nfds) {
+        memcpy(msg->fds, fds, nfds * sizeof(int));
+    }
+}
+
+bool mpqemu_msg_valid(MPQemuMsg *msg)
+{
+    if (msg->cmd >= MAX && msg->cmd < 0) {
+        return false;
+    }
+
+    if (msg->bytestream) {
+        if (!msg->data2) {
+            return false;
+        }
+        if (msg->data1.u64 != 0) {
+            return false;
+        }
+    } else {
+        if (msg->data2) {
+            return false;
+        }
+    }
+
+    /* Verify FDs. */
+    if (msg->num_fds >= REMOTE_MAX_FDS) {
+        return false;
+    }
+
+    if (msg->num_fds > 0) {
+        for (int i = 0; i < msg->num_fds; i++) {
+            if (fcntl(msg->fds[i], F_GETFL) == -1) {
+                return false;
+            }
+        }
+    }
+
+    return true;
+}
+
+void mpqemu_msg_cleanup(MPQemuMsg *msg)
+{
+    if (msg->data2) {
+        free(msg->data2);
+    }
+}