diff mbox

[RFC,v3,01/21] virtproxy: base data structures and constants

Message ID 1289870175-14880-2-git-send-email-mdroth@linux.vnet.ibm.com
State New
Headers show

Commit Message

Michael Roth Nov. 16, 2010, 1:15 a.m. UTC 
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 virtproxy.c |  136 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 virtproxy.h |   34 +++++++++++++++
 2 files changed, 170 insertions(+), 0 deletions(-)
 create mode 100644 virtproxy.c
 create mode 100644 virtproxy.h

Comments

Jes Sorensen Nov. 18, 2010, 11:06 a.m. UTC | #1 
On 11/16/10 02:15, Michael Roth wrote:
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  virtproxy.c |  136 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  virtproxy.h |   34 +++++++++++++++
>  2 files changed, 170 insertions(+), 0 deletions(-)
>  create mode 100644 virtproxy.c
>  create mode 100644 virtproxy.h
> 
> diff --git a/virtproxy.c b/virtproxy.c
> new file mode 100644
> index 0000000..8f18d83
> --- /dev/null
> +++ b/virtproxy.c
> @@ -0,0 +1,136 @@
> +/*
> + * virt-proxy - host/guest communication layer
> + *
> + * Copyright IBM Corp. 2010
> + *
> + * Authors:
> + *  Michael Roth      <mdroth@linux.vnet.ibm.com>
> + *
> + * 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 "virtproxy.h"
> +
> +#define VP_SERVICE_ID_LEN 32    /* max length of service id string */
> +#define VP_PKT_DATA_LEN 1024    /* max proxied bytes per VPPacket */
> +#define VP_CONN_DATA_LEN 1024   /* max bytes conns can send at a time */
> +#define VP_MAGIC 0x1F374059
> +
> +/* listening fd, one for each service we're forwarding to remote end */
> +typedef struct VPOForward {
> +    VPDriver *drv;
> +    int listen_fd;
> +    char service_id[VP_SERVICE_ID_LEN];
> +    QLIST_ENTRY(VPOForward) next;
> +} VPOForward;

I am really not a fan of the typedefmeharder approach you are taking in
here, but others may disagree with me.

Cheers,
Jes
Michael Roth Nov. 18, 2010, 3:35 p.m. UTC | #2 
On 11/18/2010 05:06 AM, Jes Sorensen wrote:
> On 11/16/10 02:15, Michael Roth wrote:
>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>> ---
>>   virtproxy.c |  136 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   virtproxy.h |   34 +++++++++++++++
>>   2 files changed, 170 insertions(+), 0 deletions(-)
>>   create mode 100644 virtproxy.c
>>   create mode 100644 virtproxy.h
>>
>> diff --git a/virtproxy.c b/virtproxy.c
>> new file mode 100644
>> index 0000000..8f18d83
>> --- /dev/null
>> +++ b/virtproxy.c
>> @@ -0,0 +1,136 @@
>> +/*
>> + * virt-proxy - host/guest communication layer
>> + *
>> + * Copyright IBM Corp. 2010
>> + *
>> + * Authors:
>> + *  Michael Roth<mdroth@linux.vnet.ibm.com>
>> + *
>> + * 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 "virtproxy.h"
>> +
>> +#define VP_SERVICE_ID_LEN 32    /* max length of service id string */
>> +#define VP_PKT_DATA_LEN 1024    /* max proxied bytes per VPPacket */
>> +#define VP_CONN_DATA_LEN 1024   /* max bytes conns can send at a time */
>> +#define VP_MAGIC 0x1F374059
>> +
>> +/* listening fd, one for each service we're forwarding to remote end */
>> +typedef struct VPOForward {
>> +    VPDriver *drv;
>> +    int listen_fd;
>> +    char service_id[VP_SERVICE_ID_LEN];
>> +    QLIST_ENTRY(VPOForward) next;
>> +} VPOForward;
>
> I am really not a fan of the typedefmeharder approach you are taking in
> here, but others may disagree with me.

Isn't typedef'ing structured types part of the qemu coding style 
guidelines? Or is there something beyond that I'm doing that isn't 
agreeable? The named struct + typedef approach seems to be widely used, 
I think to maintain compatibility with utility macros like these while 
still adhering to coding style in using typedefs wherever possible:

qemu-queue.h:
#define QLIST_HEAD(name, type)                       \
struct name {                                        \
         struct type *lh_first;  /* first element */  \
}

>
> Cheers,
> Jes
>
Anthony Liguori Nov. 18, 2010, 3:41 p.m. UTC | #3 
On 11/18/2010 09:35 AM, Michael Roth wrote:
>>> +/* listening fd, one for each service we're forwarding to remote 
>>> end */
>>> +typedef struct VPOForward {
>>> +    VPDriver *drv;
>>> +    int listen_fd;
>>> +    char service_id[VP_SERVICE_ID_LEN];
>>> +    QLIST_ENTRY(VPOForward) next;
>>> +} VPOForward;
>>
>> I am really not a fan of the typedefmeharder approach you are taking in
>> here, but others may disagree with me.
>
>
> Isn't typedef'ing structured types part of the qemu coding style 
> guidelines?

Yes, I think Jes was just looking for an excuse to say "typedefmeharder" :-)

Regards,

Anthony Liguori
Jes Sorensen Nov. 18, 2010, 3:51 p.m. UTC | #4 
On 11/18/10 16:41, Anthony Liguori wrote:
> On 11/18/2010 09:35 AM, Michael Roth wrote:
>>>> +/* listening fd, one for each service we're forwarding to remote
>>>> end */
>>>> +typedef struct VPOForward {
>>>> +    VPDriver *drv;
>>>> +    int listen_fd;
>>>> +    char service_id[VP_SERVICE_ID_LEN];
>>>> +    QLIST_ENTRY(VPOForward) next;
>>>> +} VPOForward;
>>>
>>> I am really not a fan of the typedefmeharder approach you are taking in
>>> here, but others may disagree with me.
>>
>> Isn't typedef'ing structured types part of the qemu coding style
>> guidelines?
> 
> Yes, I think Jes was just looking for an excuse to say "typedefmeharder"
> :-)

Actually typedefs are not listed as a must do thing in CODING_STYLE,
fortunately! It's just a really bad habit that is applied all over the
place in QEMU :(

http://www.linuxjournal.com/article/5780?page=0,2
search for typedef, for a lot of good reasoning why we shouldn't do this.

Jes
Anthony Liguori Nov. 18, 2010, 3:56 p.m. UTC | #5 
On 11/18/2010 09:51 AM, Jes Sorensen wrote:
>> Yes, I think Jes was just looking for an excuse to say "typedefmeharder"
>> :-)
>>      
> Actually typedefs are not listed as a must do thing in CODING_STYLE,
> fortunately!

That's a bug in CODING_STYLE.

typedefing structures is one of the core characteristics of QEMU coding 
style.

Regards,

Anthony Liguori

>   It's just a really bad habit that is applied all over the
> place in QEMU :(
>
> http://www.linuxjournal.com/article/5780?page=0,2
> search for typedef, for a lot of good reasoning why we shouldn't do this.
>
> Jes
>
Jes Sorensen Nov. 18, 2010, 4:03 p.m. UTC | #6 
On 11/18/10 16:56, Anthony Liguori wrote:
> On 11/18/2010 09:51 AM, Jes Sorensen wrote:
>>> Yes, I think Jes was just looking for an excuse to say "typedefmeharder"
>>> :-)
>>>      
>> Actually typedefs are not listed as a must do thing in CODING_STYLE,
>> fortunately!
> 
> That's a bug in CODING_STYLE.
> 
> typedefing structures is one of the core characteristics of QEMU coding
> style.

And here I was hoping that we could at least keep basic sanity in the
coding style document :(

typedefs are just plain wrong!

Jes
diff mbox

Patch

diff --git a/virtproxy.c b/virtproxy.c
new file mode 100644
index 0000000..8f18d83
--- /dev/null
+++ b/virtproxy.c
@@ -0,0 +1,136 @@ 
+/*
+ * virt-proxy - host/guest communication layer
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ *  Michael Roth      <mdroth@linux.vnet.ibm.com>
+ *
+ * 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 "virtproxy.h"
+
+#define VP_SERVICE_ID_LEN 32    /* max length of service id string */
+#define VP_PKT_DATA_LEN 1024    /* max proxied bytes per VPPacket */
+#define VP_CONN_DATA_LEN 1024   /* max bytes conns can send at a time */
+#define VP_MAGIC 0x1F374059
+
+/* listening fd, one for each service we're forwarding to remote end */
+typedef struct VPOForward {
+    VPDriver *drv;
+    int listen_fd;
+    char service_id[VP_SERVICE_ID_LEN];
+    QLIST_ENTRY(VPOForward) next;
+} VPOForward;
+
+/* service_id->path/port mapping of each service forwarded from remote end */
+typedef struct VPIForward {
+    VPDriver *drv;
+    char service_id[VP_SERVICE_ID_LEN];
+    QemuOpts *socket_opts;
+    QLIST_ENTRY(VPIForward) next;
+} VPIForward;
+
+/* proxied client/server connected states */
+typedef struct VPConn {
+    VPDriver *drv;
+    int client_fd;
+    int server_fd;
+    enum {
+        VP_CONN_CLIENT = 1,
+        VP_CONN_SERVER,
+    } type;
+    enum {
+        VP_STATE_NEW = 1,   /* accept()'d and registered fd */
+        VP_STATE_INIT,      /* sent init pkt to remote end, waiting for ack */
+        VP_STATE_CONNECTED, /* client and server connected */
+    } state;
+    QLIST_ENTRY(VPConn) next;
+} VPConn;
+
+typedef struct VPControlMsg {
+    enum {
+        VP_CONTROL_CONNECT_INIT = 1,
+        VP_CONTROL_CONNECT_ACK,
+        VP_CONTROL_CLOSE,
+    } type;
+    union {
+        /* tell remote end connect to server and map client_fd to it */
+        struct {
+            int client_fd;
+            char service_id[VP_SERVICE_ID_LEN];
+        } connect_init;
+        /* tell remote end we've created the connection to the server,
+         * and give them the corresponding fd to use so we don't have
+         * to do a reverse lookup everytime
+         */
+        struct {
+            int client_fd;
+            int server_fd;
+        } connect_ack;
+        /* tell remote end to close fd in question, presumably because
+         * connection was closed on our end
+         */
+        struct {
+            int client_fd;
+            int server_fd;
+        } close;
+    } args;
+} VPControlMsg;
+
+typedef struct VPPacket {
+    enum {
+        VP_PKT_CONTROL = 1,
+        VP_PKT_CLIENT,
+        VP_PKT_SERVER,
+    } type;
+    union {
+        VPControlMsg msg;
+        struct {
+            int client_fd;
+            int server_fd;
+            int bytes;
+            char data[VP_PKT_DATA_LEN];
+        } proxied;
+    } payload;
+    int magic;
+} __attribute__((__packed__)) VPPacket;
+
+struct VPDriver {
+    enum vp_context ctx;
+    int channel_fd;
+    int listen_fd;
+    CharDriverState *chr;
+    char buf[sizeof(VPPacket)];
+    int buflen;
+    QLIST_HEAD(, VPOForward) oforwards;
+    QLIST_HEAD(, VPIForward) iforwards;
+    QLIST_HEAD(, VPConn) conns;
+};
+
+static QemuOptsList vp_socket_opts = {
+    .name = "vp_socket_opts",
+    .head = QTAILQ_HEAD_INITIALIZER(vp_socket_opts.head),
+    .desc = {
+        {
+            .name = "path",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "host",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "port",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "ipv4",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "ipv6",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end if list */ }
+    },
+};
diff --git a/virtproxy.h b/virtproxy.h
new file mode 100644
index 0000000..0203421
--- /dev/null
+++ b/virtproxy.h
@@ -0,0 +1,34 @@ 
+/*
+ * virt-proxy - host/guest communication layer
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ *  Michael Roth      <mdroth@linux.vnet.ibm.com>
+ *
+ * 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 VIRTPROXY_H
+#define VIRTPROXY_H
+
+#include "qemu-common.h"
+#include "qemu-queue.h"
+
+typedef struct VPDriver VPDriver;
+
+/* wrappers for s/vp/qemu/ functions we need */
+int vp_send_all(int fd, const void *buf, int len1);
+int vp_set_fd_handler2(int fd,
+                         IOCanReadHandler *fd_read_poll,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque);
+int vp_set_fd_handler(int fd,
+                        IOHandler *fd_read,
+                        IOHandler *fd_write,
+                        void *opaque);
+
+#endif /* VIRTPROXY_H */